Skip to content

[Flyout] use container as reference for pushMinBreakpoint#9592

Merged
tsullivan merged 14 commits intoelastic:mainfrom
tsullivan:flyout/container-prop-and-pushMinBreakpoint
Apr 23, 2026
Merged

[Flyout] use container as reference for pushMinBreakpoint#9592
tsullivan merged 14 commits intoelastic:mainfrom
tsullivan:flyout/container-prop-and-pushMinBreakpoint

Conversation

@tsullivan
Copy link
Copy Markdown
Member

@tsullivan tsullivan commented Apr 14, 2026

Summary

Bug fix for EuiFlyout pushMinBreakpoint when container prop is provided.

Addresses elastic/kibana#258828

Fix: "push" type EuiFlyout has pushMinBreakpoint logic. This changes the type to overlay when the viewport size is below a given breakpoint. Typically, that breakpoint is measured against the browser viewport. When the container prop is passed to EuiFlyout, the breakpoint should be measured against the dimensions of the given container.

API Changes

component / parent prop / child change description
EuiFlyout pushMinBreakpoint Updated When container is set, the push/overlay breakpoint decision is now based on the container's width instead of the viewport width

Screenshots

Setup:

  • resizable: false
  • ownFocus: true
  • type: push
  • pushMinBreakpoint: m

Before

01 push min breakpoint fix - before

After

02 push min breakpoint fix - after

Impact Assessment

Note: Most PRs should be tested in Kibana to help gauge their Impact before merging.

  • [ ] 🔴 Breaking changes — What will break? How many usages in Kibana/Cloud UI are impacted?
  • 💅 Visual changes — May impact style overrides; could require visual testing. Explain and estimate impact.
  • [ ] 🧪 Test impact — May break functional or snapshot tests (e.g., HTML structure, class names, default values).
  • [ ] 🔧 Hard to integrate — If changes require substantial updates to Kibana, please stage the changes and link them here.

Impact level: 🟢 None / 🟢 Low / 🟡 Moderate / 🔴 High

Impact explained: When a flyout has both type="push" and a container, the point at which it flips between push and overlay mode changes. Previously it was based on viewport width; now it's based on container width. Since containers are typically narrower than the viewport, the flyout will switch to overlay sooner (at wider viewport sizes) than before.

Release Readiness

  • [ ] Documentation: {link to docs page(s)}
  • [ ] Figma: {link to Figma or issue}
  • [ ] Migration guide: {steps or link, for breaking/visual changes or deprecations}
  • [ ] Adoption plan (new features): {link to issue/doc or outline who will integrate this and where}

QA instructions for reviewer

Checklist before marking Ready for Review

Reviewer checklist

  • Approved Impact Assessment — Acceptable to merge given the consumer impact.
  • Approved Release Readiness — Docs, Figma, and migration info are sufficient to ship.

@tsullivan tsullivan marked this pull request as ready for review April 14, 2026 02:17
@tsullivan tsullivan requested a review from a team as a code owner April 14, 2026 02:17
Copilot AI review requested due to automatic review settings April 14, 2026 02:17
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes EuiFlyout’s pushMinBreakpoint behavior so that when a container is provided, the decision to render in “push” vs “overlay” mode is based on the container width (not the viewport), aligning responsive behavior with the available in-container space.

Changes:

  • Updated useIsPushed to optionally compute breakpoint logic against a provided container element’s width (via ResizeObserver + theme breakpoints).
  • Wired containerElement into useIsPushed from both unmanaged (EuiFlyoutComponent) and managed (EuiFlyoutMain) flyout entry points.
  • Added an upcoming changelog entry documenting the bug fix.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
packages/eui/src/components/flyout/manager/flyout_main.tsx Uses manager container state as the reference for pushMinBreakpoint decisions in the managed main flyout wrapper.
packages/eui/src/components/flyout/hooks.ts Extends useIsPushed to support container-width-based breakpoint evaluation and react to container resizes.
packages/eui/src/components/flyout/flyout.component.tsx Passes the resolved flyout container element into useIsPushed so push/overlay behavior matches container sizing.
packages/eui/changelogs/upcoming/9592.md Documents the pushMinBreakpoint container-scoped bug fix.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/eui/src/components/flyout/hooks.ts Outdated
Comment thread packages/eui/src/components/flyout/flyout.component.tsx Outdated
Comment thread packages/eui/src/components/flyout/manager/flyout_main.tsx
Comment thread packages/eui/src/components/flyout/hooks.ts
@acstll acstll self-requested a review April 21, 2026 07:23
Copy link
Copy Markdown
Contributor

@acstll acstll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for opening this @tsullivan! I left a couple of comments, nothing blocking.

I also have another suggestion: do you think it'd make sense to define the container prop in the flyout_manager.stories.tsx? I added it locally (here) for testing like in the snippet below (maybe there's a more correct way i'm missing):

container={() => document.querySelector('#app-main-scroll')}

Comment thread packages/eui/src/components/flyout/hooks.ts
Comment thread packages/eui/changelogs/upcoming/9592.md Outdated
@tsullivan
Copy link
Copy Markdown
Member Author

I also have another suggestion: do you think it'd make sense to define the container prop in the flyout_manager.stories.tsx?

Thanks for this suggestion, @acstll! There actually is a "Container demo" storybook in src/components/flyout/manager/flyout_containers.stories.tsx, but I consider the storybooks in the flyout/manager directory to be a bit overkill. I think we can tackle the overkill, and give more highlight to the features that make the Flyout System great.

  1. There is a src/components/flyout/manager/flyout_containers.stories.tsx ("Container Demo") that highlights the container API of flyouts.
  2. There are two other "advanced" stories, both in src/components/flyout/manager/flyout_sessions.stories.tsx
    • "Multi-session example": demonstrates history management features
    • "Multi-root sync": demonstrates synchronized state across multiple React mount points.
  3. The src/components/flyout/manager/flyout_manager.stories.tsx ("Playground") storybook is a simple demonstration of child flyouts and allows toggling different props.

I think we can eventually get rid of the storybooks from (2) and maybe (3):

  • "Multi-session example": the history management features should be refactored out of EUI and moved to Kibana. There's an issue in the private kibana-team repo tracking this.
  • "Multi-root sync": this proves that a requirement is working - it should be replaced with unit tests.
  • "Playground": the "Container Demo" also shows off child flyouts. If the "Container Demo" storybook also does an adequate job of allowing toggling the different props, the current "Playground" storybook would be redundant and could be removed.

@elasticmachine
Copy link
Copy Markdown
Collaborator

💚 Build Succeeded

History

@elasticmachine
Copy link
Copy Markdown
Collaborator

💚 Build Succeeded

History

Copy link
Copy Markdown
Contributor

@acstll acstll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Thanks for addressing the feedback and for contributing the fix!

@acstll
Copy link
Copy Markdown
Contributor

acstll commented Apr 22, 2026

I think we can eventually get rid of the storybooks from (2) and maybe (3):

thanks for the detailed explanation @tsullivan — i don't have a strong opinion, but i do agree removing them for the reasons you're outlining seems like the right move 👍

the "container demo" was useful for me in this case, it allowed me to test the changes locally without using Kibana 😅

@tsullivan tsullivan merged commit 6361771 into elastic:main Apr 23, 2026
5 checks passed
acstll added a commit to elastic/kibana that referenced this pull request Apr 28, 2026
## Dependency updates

- `@elastic/eui` - v114.2.0 ⏩ v114.3.0

---

## Package updates

## [`v114.3.0`](https://github.com/elastic/eui/releases/v114.3.0)

- Updated `productDashboard` icon.
([#9607](elastic/eui#9607))
- Updated `EuiStepsHorizontal` to prevent steps being clickable when
`status="disabled"` ([#9574](elastic/eui#9574))

**Bug fixes**

- Fixed broken SVG for `chartPie` icon.
([#9607](elastic/eui#9607))
- Fixed a bug in `EuiDataGrid` that caused the scroll position to reset
when using middle mouse button to scroll the container.
([#9613](elastic/eui#9613))
- Fixed `EuiFlyout` to compare `pushMinBreakpoint` against the
container's width, instead of the viewport width, when the `container`
prop is provided. This ensures app-scoped flyouts switch between push
and overlay modes based on the space actually available inside their
container. ([#9592](elastic/eui#9592))

**Accessibility**

- Fixed duplicate screen reader output for `EuiStepsHorizontal`
([#9574](elastic/eui#9574))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants